Let the application own the MUSE_COMPILE_USE_PCH option - #254
luapmartin wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe top-level Merge Risk: ⚪ Minimal · up to This change moves ownership of the PCH build option to the application and keeps the standalone framework build explicitly disabled; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The framework changes remove the option declaration and add standalone-build defaults, but linked issue [ Resolution Add the required MUSE_COMPILE_USE_PCH option to Audacity's root CMakeLists.txt before including the framework options, or provide explicit evidence that this PR is only the framework-side prerequisite and that the linked issue is intentionally resolved by a separate consumer-repository change.
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
015f2c3 to
82dc7f3
Compare
82dc7f3 to
a0e9f48
Compare
a0e9f48 to
9b35bd4
Compare
|
Closing after discussion with Igor: framework options stay declared in the framework, and an application overrides one by setting it before including the framework. The two options still declared only app-side (MUSE_COMPILE_USE_UNITY, MUSE_COMPILE_USE_SHARED_LIBS_IN_DEBUG) get folded into the framework in a follow-up PR. |
Tracker with the merge order: audacity/audacity#11898.
Removes the
MUSE_COMPILE_USE_PCHdeclaration fromMuseDeclareOptions.cmake. The framework keeps reading the option; the application declares it, likeMUSE_COMPILE_USE_UNITY, and no longer has to declare it first to control the default (audacity/audacity#11795).An application that does not declare it builds without PCH, silently. audacity/audacity#11900 and musescore/MuseScore#34791 (one line each) must merge before each application bumps its
musesubmodule past this PR. This PR itself can merge before or after them.The standalone
CMakeLists.txtdeclares both options OFF, so a standalone framework build is strict by default (possible since #256). The unit-test job still forces unity ON inninja_build_utests.sh; the CI net for the fully strict configuration is audacity/audacity#12246.Build configuration
audacity: audacity/audacity/master
audacity platforms: linux_x64
musescore: musescore/MuseScore/main
musescore platforms: linux_x64